Allow calls to vararg functions to be inlined if the number of arguments are known at compile-time #2007
Conversation
…ile time Contains: Changes to Compiler +2 tests to replace 1 test
|
Closing this PR, I need to do some more investigation. I was a little surprised when I made the initial changes and found that everything seemed to just work without changing any of the actual function inlining code, but I guess it really was too good to be true given the failing checks. I thought I ran the full suite locally but I suppose not |
|
I'm not sure whether some of these test failures are false-positives or not. On the Windows action, there was somehow a Type Inference test failing. On the Mac OS action, there's no stacktrace to the call-site of |
|
Don't forget to run conformance in -O2, --codegen -O2 and with fflags=true as well, especially when adding -O2 feature. |
PhoenixWhitefire
changed the title
Aha, thanks for pointing that out. I was not aware that I had to do so manually. I can reproduce the failing tests locally now. |
Fixes the simplest case of:
---
local function foo(...)
print(...)
end
foo("Hello, World!")
---
Does not pass tests/Issue snippet
Instead of blanket-disabling inlining for vararg functions, with these changes inlining is evaluated at each call-site instead. Inlining can now succeed if the number of arguments passed to the function is known at compile-time (fixedret). I ran the Unit and Conformance tests and found no issues. For reasons beyond me, the Benchmark just showed "FAILURE" for every test, but these changes shouldn't have any significant regressions.
Additionally, I replaced the vararg inlining failure test with 2 new tests, which validate the two possible code paths for these changes.
Implements: #1696